odhcpd: improve odhcpd_urandom()

First, note that not a single caller checks the return value - which is
quite reasonable. What are they supposed to do with a failure?

Second, none of the callers do anything that's *really*
security-sensitive, the closest we have is the force reconf nonce, and
that is blorted out over the network, so it's really a best-effort kind
of thing.

Third, odhcpd_urandom() currently doesn't check if it e.g. got
interrupted by a signal.

So, simplify and modernize this a bit by using getrandom(), which allows
us to skip one fd, and which avoids syscalls by using the vDSO approach
instead. Also, check for things like signal interrupts (don't really
happen on calls for entropy < 256 bytes, but still). And make a
reasonable effort, but not much more.

Signed-off-by: David Härdeman <[email protected]>
Link: https://github.com/openwrt/odhcpd/pull/285
Signed-off-by: Álvaro Fernández Rojas <[email protected]>

diff --git a/src/odhcpd.c b/src/odhcpd.c
index ef67acc630a1c142be0b29eab29179dccd3333dd..0f314bf7a61fb75715efbc0fc8fe729f0fd9e0fa 100644 (file)
--- a/src/odhcpd.c
+++ b/src/odhcpd.c
@@ -39,12 +39,12 @@
 #include <sys/types.h>
 #include <sys/wait.h>
 #include <sys/syscall.h>
+#include <sys/random.h>
 
 #include <libubox/uloop.h>
 #include "odhcpd.h"
 
 static int ioctl_sock = -1;
-static int urandom_fd = -1;
 
 void __iflog(int lvl, const char *fmt, ...)
 {
@@ -148,13 +148,9 @@ int main(int argc, char **argv)
        }
 
        ioctl_sock = socket(AF_INET, SOCK_DGRAM | SOCK_CLOEXEC, 0);
-
        if (ioctl_sock < 0)
                return 4;
 
-       if ((urandom_fd = open("/dev/urandom", O_RDONLY | O_CLOEXEC)) < 0)
-               return 4;
-
        signal(SIGUSR1, SIG_IGN);
        signal(SIGINT, sighandler);
        signal(SIGTERM, sighandler);
@@ -542,11 +538,33 @@ void odhcpd_process(struct odhcpd_event *event)
        odhcpd_receive_packets(&event->uloop, 0);
 }
 
-int odhcpd_urandom(void *data, size_t len)
+void odhcpd_urandom(void *data, size_t len)
 {
-       return read(urandom_fd, data, len);
-}
+       static bool warned_once = false;
 
+       while (true) {
+               ssize_t r;
+
+               if (len == 0)
+                       return;
+
+               r = getrandom(data, len, GRND_INSECURE);
+               if (r < 0) {
+                       if (errno == EINTR)
+                               continue;
+
+                       if (!warned_once) {
+                               error("getrandom(): %m");
+                               warned_once = true;
+                       }
+
+                       return;
+               }
+
+               len -= r;
+               data = (uint8_t *)data + r;
+       }
+}
 
 time_t odhcpd_time(void)
 {

diff --git a/src/odhcpd.h b/src/odhcpd.h
index ae0e6f5dc61be21dde462c852ce78654ae5e5fc4..02babc5e31a3df2cdf9b5cad91e24bf5fdad74da 100644 (file)
--- a/src/odhcpd.h
+++ b/src/odhcpd.h
@@ -514,7 +514,7 @@ int odhcpd_get_interface_config(const char *ifname, const char *what);
 int odhcpd_get_mac(const struct interface *iface, uint8_t mac[6]);
 int odhcpd_get_flags(const struct interface *iface);
 struct interface* odhcpd_get_interface_by_index(int ifindex);
-int odhcpd_urandom(void *data, size_t len);
+void odhcpd_urandom(void *data, size_t len);
 
 void odhcpd_run(void);
 time_t odhcpd_time(void);